feat(web): validation of the token address for ERC20/721/1155 types #2052
Conversation
WalkthroughAdds token-gate address validation for gated dispute kits: new validation hooks, UI feedback in the Court page, state synchronization into dispute context, Next button gating logic, and a small API change to extraDataToTokenInfo and IGatedDisputeData to carry validation state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CourtPage
participant ValidationHook
participant EthereumClient
participant DisputeContext
participant NextButton
User->>CourtPage: Enter token address / toggle ERC-1155
CourtPage->>ValidationHook: Request validation (address, type)
ValidationHook->>EthereumClient: Call contract balanceOf / check interface
EthereumClient-->>ValidationHook: Respond (success / revert / error)
ValidationHook-->>CourtPage: Return isValid / error / loading
CourtPage->>DisputeContext: Update isTokenGateValid (null/true/false)
CourtPage-->>User: Show spinner / check / cross and message
NextButton->>DisputeContext: Read isTokenGateValid + disputeKitId
NextButton-->>User: Enable or disable navigation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
jaybuidl
requested review from
tractorss and
kemuru
and removed request for
a team
jaybuidl
force-pushed
the
feat/gated-dk-address-validation
branch
from
8a3862f to
f1fe7c9
Compare
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
24-25: Consider using optional chaining for cleaner code.- const gatedData = disputeData.disputeKitData as IGatedDisputeData; - if (!gatedData?.tokenGate?.trim()) return true; // No token address provided, so valid + const gatedData = disputeData.disputeKitData as IGatedDisputeData; + if (!gatedData.tokenGate?.trim()) return true; // No token address provided, so validweb/src/hooks/useTokenAddressValidation.ts (2)
169-171: Consider preserving error details for debugging.The catch block discards the original error which might contain useful information for debugging contract-specific issues.
- } catch { - throw new Error(`Address does not implement ${tokenType} interface`); + } catch (error) { + console.debug(`Token validation failed for ${debouncedAddress}:`, error); + throw new Error(`Address does not implement ${tokenType} interface`); }
202-210: Error message handling could be more robust.The error message parsing relies on string matching which might be fragile if error messages change.
Consider using error codes or more structured error handling if the contract interaction library supports it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/src/context/NewDisputeContext.tsx(1 hunks)web/src/hooks/useTokenAddressValidation.ts(1 hunks)web/src/pages/Resolver/NavigationButtons/NextButton.tsx(3 hunks)web/src/pages/Resolver/Parameters/Court.tsx(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
🔇 Additional comments (7)
web/src/context/NewDisputeContext.tsx (1)
64-64: LGTM! Well-designed validation state property.The tri-state boolean (
null= not validated,false= invalid,true= valid) is an appropriate design choice for tracking asynchronous validation status.web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
20-29: LGTM! Validation logic is well-structured.The memoized validation check correctly handles all cases: non-gated kits, empty token addresses, and validated addresses.
web/src/pages/Resolver/Parameters/Court.tsx (3)
1-1: Import statement updated correctly.Added
useEffectimport which is used for validation state synchronization.
90-168: Excellent UI feedback components for validation.The styled components provide clear visual feedback with loading spinner, success checkmark, and error cross. The color scheme follows standard UX patterns (green for success, red for error).
327-336: Well-implemented validation feedback UI.The conditional rendering of validation status with appropriate messages for each state (validating, error, success) provides excellent user experience.
web/src/hooks/useTokenAddressValidation.ts (2)
122-129: Good implementation of debouncing.The 500ms debounce delay is appropriate for reducing unnecessary network calls while maintaining responsive UX.
150-152: Appropriate caching and retry configuration.The 5-minute cache time and single retry with 1-second delay provide a good balance between performance and user experience.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
web/src/utils/extradataToTokenInfo.ts (1)
24-29: Avoid Buffer in the browser; use viem’s bytesToHexUsing Buffer can break in web builds if polyfills aren’t present. viem exposes bytesToHex which is portable and simpler.
Apply these changes:
-import { Address, hexToBytes } from "viem"; +import { Address, hexToBytes, bytesToHex } from "viem"; @@ - const packed = BigInt("0x" + Buffer.from(packedBytes).toString("hex")); + const packed = BigInt(bytesToHex(packedBytes)); @@ - const tokenId = BigInt("0x" + Buffer.from(tokenIdBytes).toString("hex")); + const tokenId = BigInt(bytesToHex(tokenIdBytes));
♻️ Duplicate comments (1)
web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
44-45: Court step gating is correct and enforces mandatory token address for gated kitsDisabling Next when the token gate is missing/invalid, courtId is unset, or disputeKitId is missing aligns with the requirement that token address is mandatory for token-gated dispute kits.
🧹 Nitpick comments (3)
web/src/utils/extradataToTokenInfo.ts (1)
19-20: Be explicit: return undefined instead of bare returnReturning undefined explicitly improves readability and satisfies stricter linters (e.g., consistent-return).
- if (extraDataBytes.length < 160) { - return; - } + if (extraDataBytes.length < 160) { + return undefined; + }web/src/pages/Resolver/NavigationButtons/NextButton.tsx (2)
20-30: Simplify and de-stale: compute gating validity without useMemo and remove unnecessary type assertionThe computation is trivial and better evaluated directly each render. This avoids potential staleness if the context mutates the object in place and removes an unnecessary cast (type is already narrowed by the "type" discriminator).
- // Check gated dispute kit validation status - const isGatedTokenValid = React.useMemo(() => { - if (!disputeData.disputeKitData || disputeData.disputeKitData.type !== "gated") return true; - - const gatedData = disputeData.disputeKitData as IGatedDisputeData; - if (!gatedData?.tokenGate?.trim()) return false; // No token address provided, so invalid - - // If token address is provided, it must be validated as valid ERC20 - return gatedData.isTokenGateValid === true; - }, [disputeData.disputeKitData]); + // Check gated dispute kit validation status + const isGatedTokenValid = + disputeData.disputeKitData?.type !== "gated" + ? true + : Boolean(disputeData.disputeKitData.tokenGate?.trim()) && + disputeData.disputeKitData.isTokenGateValid === true;Optional (if ERC-1155 requires a tokenId to proceed): additionally gate on tokenId presence for ERC-1155:
- : Boolean(disputeData.disputeKitData.tokenGate?.trim()) && - disputeData.disputeKitData.isTokenGateValid === true; + : Boolean(disputeData.disputeKitData.tokenGate?.trim()) && + disputeData.disputeKitData.isTokenGateValid === true && + (!disputeData.disputeKitData.isERC1155 || + String(disputeData.disputeKitData.tokenId ?? "").trim() !== "");
24-26: Remove the type assertion; discriminated union already narrowsAfter checking type !== "gated", TS can narrow disputeKitData to IGatedDisputeData when type === "gated", making the cast redundant.
No separate diff needed if you apply the simplification in the previous comment; otherwise:
- const gatedData = disputeData.disputeKitData as IGatedDisputeData; + const gatedData = disputeData.disputeKitData;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/pages/Resolver/NavigationButtons/NextButton.tsx(3 hunks)web/src/utils/extradataToTokenInfo.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
PR: kleros/kleros-v2#1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/pages/Resolver/NavigationButtons/NextButton.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/pages/Resolver/NavigationButtons/NextButton.tsx
📚 Learning: 2024-10-29T10:13:04.524Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1729
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:69-69
Timestamp: 2024-10-29T10:13:04.524Z
Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when the button is disabled, the associated logic won't be reached, and certain code paths may exist for TypeScript purposes.
Applied to files:
web/src/pages/Resolver/NavigationButtons/NextButton.tsx
🧬 Code Graph Analysis (1)
web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
web/src/context/NewDisputeContext.tsx (1)
IGatedDisputeData(59-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
🔇 Additional comments (2)
web/src/utils/extradataToTokenInfo.ts (1)
13-16: ```diff
- Public API now returns undefined — verify all call sites handle it
- Verify callers of extraDataToTokenInfo handle undefined result
Findings: Only one usage in web/src/pages/Resolver/Landing/index.tsx – inside a useMemo:
const gatedTokenInfo = useMemo(() => { if (isUndefined(extradata)) return return extraDataToTokenInfo(extradata) }, [roundData])
- The code guards only against a missing
extradatainput, not against the function returningundefinedwhen the data isn’t a gated disputeKit.- In practice, both “no input” and “non-gated data” yield
undefinedhere, so downstream code sees the same value.- Confirm that wherever
gatedTokenInfogets used, the component tolerates it beingundefined(e.g. via conditional rendering or optional chaining).Action items:
• In web/src/pages/Resolver/Landing/index.tsx, audit any usage ofgatedTokenInfoto ensure you check forundefinedbefore accessing its properties.
• If additional call sites ofextraDataToTokenInfoexist elsewhere in the codebase, repeat this check.web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
7-8: LGTM on importing IGatedDisputeData for accurate narrowingImporting the discriminated union member improves type safety across components relying on disputeKitData.
PR-Codex overview
This PR enhances the token validation functionality in the dispute management system. It introduces checks for ERC20, ERC721, and ERC1155 tokens, updating the context and UI to reflect the validation status of token gates in gated dispute kits.
Detailed summary
isTokenGateValidproperty toNewDisputeContext.extraDataToTokenInfoto returnundefinedfor non-gated dispute kits.NextButtonto include gated token validation logic.useERC20ERC721ValidationanduseERC1155Validationfor token address validation.Courtcomponent to manage token validation state and UI feedback.Courtcomponent.Summary by CodeRabbit
New Features
Bug Fixes